New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Touch Manager #423
Touch Manager #423
Conversation
Current coverage is 69.51% (diff: 74.64%)@@ develop #423 diff @@
==========================================
Files 281 289 +8
Lines 9376 9589 +213
Methods 2585 2636 +51
Messages 0 0
Branches 603 623 +20
==========================================
+ Hits 6507 6666 +159
- Misses 2581 2618 +37
- Partials 288 305 +17
|
… be easier for developers.
…1 for SDLTouchZero.
… Core, we will use device's timestamp.
@joeljfischer start reviewing the tests please when you get a chance. |
@@ -0,0 +1,25 @@ | |||
// | |||
// SDLTouchManagerListener.h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Listener is the Java convention I believe. It should be named -Delegate
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If that's the case, we should be modifying all other Protocols that use -Listener
, I was simply trying to keep it consistent 👎
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, definitely. It's just a breaking change to do so.
…esulting in no panning callbacks.
We already know timers don't work properly in the background. Does the touch manager explicitly handle that case? For example, it could watch for background notifications and stop timers, watch for foreground and restart, but it would also have to notify the developer that it's not going to work in the background. |
Video streaming doesn't work in the background, so I don't think we really need to be concerned with the timers. |
* @abstract | ||
* First touch of a pinch gesture. | ||
*/ | ||
@property (nonatomic, copy) SDLTouch* firstTouch; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These aren't copyable, I don't think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right! This was when we were using c-structs instead of objects.
…o we can end timers.
…nstead of the Proxy.
I'm seeing a few test failures after the changes |
LGTM |
# Conflicts: # SmartDeviceLink/SDLStreamingMediaManager.h # SmartDeviceLink/SDLStreamingMediaManager.m
Fixes #402
This PR is ready for review.
Risk
This PR makes minor API changes.
Testing Plan
Tests need to be written to make sure that we are correctly calculating pinch, pan, single and double touches.
Summary
This PR introduces the Touch Manager, which is, simply, a wrapper around the
onOnTouchEvent
notification that we receive from Core. TM handles the logic for determining when a single or double tap has occurred, as well as panning and pinching. Some of these properties are changeable for the developer, but the main objective here is to develop a standard implementation so developers can focus on creating applications, not on logic to handle touching events.The Touch Manager is only valid for applications utilizing video streaming.
There are a few classes/structs/headers that have been added:
onOnTouchEvent
notification, and calling theSDLTouchManagerListener
's protocol.SDLTouchEvent
in an easier to consumer manner, as well as some functions to assist in comparisons and validity.SDLTouch
structs. This also contains functions for assisting in comparisons and validity.Changelog
Enhancements
Tasks Remaining: